-
Notifications
You must be signed in to change notification settings - Fork 66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce support for error specific retry policy overrides #331
base: master
Are you sure you want to change the base?
Conversation
// Customize backoff settings for specific errors. | ||
// Similar to 'Non-Retryable', the key should precisely match the error *type*. | ||
map<string, BackoffSettings> error_backoff_overrides = 6; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Customize backoff settings for specific errors. | |
// Similar to 'Non-Retryable', the key should precisely match the error *type*. | |
map<string, BackoffSettings> error_backoff_overrides = 6; | |
// Customize backoff settings for specific errors. | |
// Similar to 'Non-Retryable', the key should match the error *type* | |
// in a manner appropriate for the SDK language. EX: `ErrorTypeName` for Go. | |
map<string, BackoffSettings> error_backoff_overrides = 6; |
The problem here is we really aren't "precise", at least not in every language, and it's language dependent. In Go for example non-retryable errors are specified using only the type name rather than a fully qualified type name which would be much more precise. So I'm not sure the word "precisely" belongs here, at least not in all cases.
We'll have to make sure in the SDKs that we're clear in the docstring about what is actually being matched.
// of error backoff overrides in the RetryPolicy message. | ||
message BackoffSettings { | ||
// Interval of the first retry. If retryBackoffCoefficient is 1.0 then it is used for all retries. | ||
google.protobuf.Duration initial_interval = 1 [(gogoproto.stdduration) = true]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to remove these gogoproto annotations; they're no longer supported
Per off-PR discussion, I think we need to discuss use cases and how we can more generically allow retry policy customization instead of specific retry policy overrides for error types which is a very specific customization. |
What changed?
error specific backoff added to RetryPolicy
Why?
answering OSS-361: Different retry options based on failure type
Breaking changes
No